fix(js): respect the @nx/js/typescript plugin filters when syncing references - #36484
Draft
FrozenPandaz wants to merge 2 commits into
Draft
fix(js): respect the @nx/js/typescript plugin filters when syncing references#36484FrozenPandaz wants to merge 2 commits into
FrozenPandaz wants to merge 2 commits into
Conversation
…ferences `typescript-sync` picked root `tsconfig.json` references from the project graph alone, ignoring the `include`/`exclude` on the plugin's own `nx.json` registrations. Projects the plugin was told to skip were still demanded as root references — pulling nested standalone workspaces, which have their own lockfile and install, into the parent repo's TypeScript build graph. Filters candidates through `findMatchingConfigFiles`, the same matcher Nx uses to decide which files a plugin's `createNodes` sees, so sync and inference agree by construction rather than through two glob implementations that can drift. Registrations are additive, since one may exclude a directory another includes. It fails open: a bare string registration, or no registration at all, filters nothing. Also moves the composite check into the pass that records missing references. It previously ran only when writing, so a non-composite project could be named as the reason a workspace was out of sync while being filtered out before any write — an unsatisfiable message. In nrwl/nx this surfaced as 13 reported missing references where only 1 was ever a real candidate. Fixes NXC-4734
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for nx-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit 57b5e5c
☁️ Nx Cloud last updated this comment at |
…ferences [Self-Healing CI Rerun]
Contributor
There was a problem hiding this comment.
Nx Cloud has identified a flaky task in your failed CI:
🔂 Since the failure was identified as flaky, we triggered a CI rerun by adding an empty commit to this branch.
🔔 Heads up, your workspace has pending recommendations ↗ to auto-apply fixes for similar failures.
🎓 Learn more about Self-Healing CI on nx.dev
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Current Behavior
@nx/js:typescript-syncdecides which projects belong in the roottsconfig.jsonreferences from the project graph alone. It never consults theinclude/excludeon the@nx/js/typescriptplugin's ownnx.jsonregistrations, so a project the plugin was explicitly told to skip can still be demanded as a root reference.This repo already declares that intent:
{ "plugin": "@nx/js/typescript", "exclude": ["examples/angular-rspack/**/*", "examples/react/**/*", "nx-dev/**/*", "e2e/**/*"] }Yet adding
"composite": truetoexamples/react/basic/tsconfig.base.jsonand runningnx sync:checkreports the workspace out of sync, demanding root references for 13 example projects. Those examples are deliberately standalone pnpm workspaces — own lockfile, own install,link:deps — so referencing them couples the parent repo'stsc --buildto a separately installed workspace.A second, independent bug makes the report misleading.
changedFiles, which drives both the out-of-sync verdict and the message, is populated before the composite filter is applied — the filter only runs when deciding what to write. So 12 of those 13 entries were phantom: theangular-rspackexamples are not composite and would never have been written. Onlyexamples/react/basicwas ever a real candidate.Expected Behavior
Sync only considers projects the plugin actually manages, and only reports references it could actually write.
Candidates are filtered through
findMatchingConfigFiles— the same matcher Nx uses to decide which files a plugin'screateNodessees — so sync and inference agree by construction rather than via two glob implementations that can drift. Matching is against the tsconfig path, since plugin patterns likeexamples/react/**/*are written against files.Three behaviors worth calling out:
e2e/**/*, the second includes it. A single-registration read would have dropped every e2e project's reference."@nx/js/typescript") or no registration at all filters nothing. This is core behavior for every Nx workspace, and narrowing on absent config would be a silent regression.Verification
Two of the five new tests fail without the fix — one per bug. The other three are regression guards proving the filter doesn't over-filter (second registration, bare string, unregistered plugin); those pass either way by design.
End-to-end against this repo, with
composite: trueadded toexamples/react/basic:@nx/js23.2.0-beta.2(no fix)nx run-many -t lint,test -p jspasses — 821 tests, including the 43 pre-existingtypescript-syncspecs.Note that
nx sync:checkin this repo runs the published@nx/js, so it cannot exercise this change until release; the A/B above drives the built generator directly against the real workspace.Related Issue(s)
Fixes NXC-4734
View session information ↗